Remove duplicate ci-builder entry from config #24
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces documentation updates to the Optimism project's README files. The main Changes
The changes are purely documentation-related and do not introduce any code modifications or functional alterations to the project. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
indexer/README.md (2)
43-43: Consider expanding API documentationThe text improvement is good, but given the "TBD docs" note, consider adding more detailed API documentation.
Would you like me to help create a detailed API documentation template?
63-63: Fix abbreviation formatting and grammarTwo minor issues to address:
- The abbreviation "ie." should be "i.e." (with periods)
- The word "adjacent" is correctly used instead of "adjacently"
-The indexer service runs a lightweight health server adjacent to the main service. The health server exposes a single endpoint `/healthz` that can be used to check the health of the indexer service. The health assessment doesn't check dependency health (ie. database) +The indexer service runs a lightweight health server adjacent to the main service. The health server exposes a single endpoint `/healthz` that can be used to check the health of the indexer service. The health assessment doesn't check dependency health (i.e., database)🧰 Tools
🪛 LanguageTool
[uncategorized] ~63-~63: The abbreviation “i.e.” (= that is) requires two periods.
Context: ...ssment doesn't check dependency health (ie. database) but rather checks the health ...(I_E)
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(0 hunks)indexer/README.md(3 hunks)
💤 Files with no reviewable changes (1)
- README.md
🧰 Additional context used
🪛 LanguageTool
indexer/README.md
[uncategorized] ~63-~63: The abbreviation “i.e.” (= that is) requires two periods.
Context: ...ssment doesn't check dependency health (ie. database) but rather checks the health ...
(I_E)
🔇 Additional comments (3)
indexer/README.md (3)
8-8: LGTM: Improved clarity in configuration documentation
The revised wording better explains the configuration parameters for RPC endpoints and polling settings.
66-66: LGTM: Improved grammar in metrics documentation
The subject-verb agreement correction makes the documentation more professional.
11-11: Verify referenced paths in testing instructions
The testing instructions reference important dependencies and paths. Let's verify their accuracy.
Also applies to: 13-13
✅ Verification successful
Let me gather more information to verify the testing instructions.
Let me check the actual content of the Makefile to verify the test command.
Testing instructions in README.md are accurate and verified
The instructions stating "All tests can be run by running make test from the /indexer directory" are correct as:
- The
/indexerdirectory exists and contains a Makefile - The Makefile has a properly configured
testtarget that runsgo test -v ./... - Both unit tests and e2e tests are present in the codebase:
- Unit tests are scattered across various packages (e.g.,
api_test.go,models_test.go) - E2e tests are in
indexer/e2e_tests/directory
- Unit tests are scattered across various packages (e.g.,
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the existence of referenced paths and make targets
fd -t f "README.md" op-e2e/
fd -t f "devnet-allocs" -x grep -l "devnet-allocs"
Length of output: 167
Script:
#!/bin/bash
# Check if Makefile exists and verify test target
fd Makefile indexer/
fd Makefile indexer/ -x grep -l "test:"
# Check the directory structure
fd . indexer/ -t d -d 1
Length of output: 329
Script:
#!/bin/bash
# Check the test target in Makefile
cat indexer/Makefile
# Check for test files
fd "_test.go" indexer/
Length of output: 1068
indexer/README.md
Outdated
|
|
||
| ### Run the Indexer (docker-compose) | ||
| The local [docker-compose.go](https://github.com/ethereum-optimism/optimism/blob/develop/indexer/docker-compose.yml) file spins up **index, api, postgres, prometheus and grafana** services. The `indexer.toml` file is setup for the local devnet. To run against a live network, update the `indexer.toml` with the desired configuration. | ||
| The local [docker-compose.go](https://github.com/ethereum-optimism/optimism/blob/develop/ops-bedrock/docker-compose.yml) file spins up **index, api, postgres, prometheus and grafana** services. The `indexer.toml` file is set up for the local devnet. To run against a live network, update the `indexer.toml` with the desired configuration. |
There was a problem hiding this comment.
Fix incorrect file extension in docker-compose link
The link to docker-compose file has an incorrect extension (.go instead of .yml).
-The local [docker-compose.go](https://github.com/ethereum-optimism/optimism/blob/develop/ops-bedrock/docker-compose.yml)
+The local [docker-compose.yml](https://github.com/ethereum-optimism/optimism/blob/develop/ops-bedrock/docker-compose.yml)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| The local [docker-compose.go](https://github.com/ethereum-optimism/optimism/blob/develop/ops-bedrock/docker-compose.yml) file spins up **index, api, postgres, prometheus and grafana** services. The `indexer.toml` file is set up for the local devnet. To run against a live network, update the `indexer.toml` with the desired configuration. | |
| The local [docker-compose.yml](https://github.com/ethereum-optimism/optimism/blob/develop/ops-bedrock/docker-compose.yml) file spins up **index, api, postgres, prometheus and grafana** services. The `indexer.toml` file is set up for the local devnet. To run against a live network, update the `indexer.toml` with the desired configuration. |
There was a problem hiding this comment.
It is correctly noticed. I fixed
There was a problem hiding this comment.
@savvar9991, thanks for confirming that the issue has been resolved. Let me know if there's anything else I can assist with!
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
-The local [docker-compose.go](https://github.com/ethereum-optimism/optimism/blob/develop/ops-bedrock/docker-compose.yml) +The local [docker-compose.yml](https://github.com/ethereum-optimism/optimism/blob/develop/ops-bedrock/docker-compose.yml)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
indexer/README.md (3)
11-13: Enhance readability with better formattingConsider structuring the prerequisites as a bulleted list for better readability:
All tests can be run by running `make test` from the `/indexer` directory. This will run all unit and e2e tests. -> **NOTE:** Successfully running the E2E tests requires spinning up a local devnet via [op-e2e](https://github.com/ethereum-optimism/optimism/tree/develop/op-e2e) and pre-populating it with necessary bedrock genesis state. This genesis state is generated by invoking the `make devnet-allocs` target from the root of the optimism monorepo before running the indexer tests. More information on this can be found in the [op-e2e README](../op-e2e/README.md). A postgres database through pwd-less user $DB_USER env variable on port 5432 must be available as well. +> **Prerequisites:** +> * Local devnet via [op-e2e](https://github.com/ethereum-optimism/optimism/tree/develop/op-e2e) +> * Bedrock genesis state (generate using `make devnet-allocs` from monorepo root) +> * Postgres database with password-less user specified in $DB_USER (port 5432) +> +> For more details, see the [op-e2e README](../op-e2e/README.md).
43-43: Track missing API documentationThe "TBD" marker indicates that API endpoint documentation is pending. This should be tracked to ensure it gets completed.
Would you like me to create a GitHub issue to track the missing API documentation task?
63-63: Fix abbreviation formattingThe abbreviation "ie." should be properly formatted as "i.e." (with periods).
-The health assessment doesn't check dependency health (ie. database) but rather checks the health of the indexer service itself. +The health assessment doesn't check dependency health (i.e., database) but rather checks the health of the indexer service itself.🧰 Tools
🪛 LanguageTool
[uncategorized] ~63-~63: The abbreviation “i.e.” (= that is) requires two periods.
Context: ...ssment doesn't check dependency health (ie. database) but rather checks the health ...(I_E)
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
indexer/README.md(3 hunks)
🧰 Additional context used
🪛 LanguageTool
indexer/README.md
[uncategorized] ~63-~63: The abbreviation “i.e.” (= that is) requires two periods.
Context: ...ssment doesn't check dependency health (ie. database) but rather checks the health ...
(I_E)
🔇 Additional comments (3)
indexer/README.md (3)
8-8: LGTM: Clear and accurate configuration documentation
The explanation of the configuration parameters is well-written and technically accurate.
16-16: LGTM: Correct docker-compose file reference
The docker-compose file path and extension are now correctly specified.
66-66: LGTM: Clear metrics documentation
The metrics documentation clearly explains the exposure of Prometheus metrics through the health server.
|
This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
|
REMOVE STALE |
ci-builder